Skip to content

Adds support for nCrunch. #825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 17, 2014
Merged

Adds support for nCrunch. #825

merged 1 commit into from
Sep 17, 2014

Conversation

MicahZoltu
Copy link
Contributor

Adds necessary files and ignores so that nCrunch can automatically run the tests.

Note: The way the tests are setup right now are very brittle when run in parallel. Just looking at the test failures, it appears to be a problem with file locking and multiple tests manipulating the same on-disk resources. I would encourage resolving this (though I can appreciate that it is low priority). The first thing I would try is to have each test make a copy of the resource it wants to manipulate, work with it, then delete its copy when it is done. This would allow several tests to simultaneously manipulate the same underlying set of files without conflict.

@MicahZoltu
Copy link
Contributor Author

Note: The supplied nCrunch files are setup to run sequentially so they pass reliably.

@nulltoken
Copy link
Member

Hey. Thanks for this. Unfortunately I don't own a nCrunch licence so I cannot test this. Before merging this in, I'd feel better to get a 👍 from a second pair of eye (@dahlbyk would you be a nCrunch user?).

Regarding the tests, IO perfs are not that great on Windows (Full test requires 5x more time to run on Windows when compared to Linux). So we tried to reduce the copying of the test repositories to the bare minimum in order to reduce the pain. However, your point is a very valid one. Would be opened to create a separate issue in order to log this?

@MicahZoltu
Copy link
Contributor Author

#826

@nulltoken
Copy link
Member

@nulltoken
Copy link
Member

@Zoltu I've downloaded a trial version of NCrunch in order to start working on this.

<TestName>LibGit2Sharp.Tests.ShadowCopyFixture.CanProbeForNativeBinariesFromAShadowCopiedAssembly</TestName>
</NamedTestSelector>
</IgnoredTests>
<AdditionalFilesToInclude>Resources\**;Resources\**.*</AdditionalFilesToInclude>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need both paths? Couldn't this be only?

<AdditionalFilesToInclude>Resources\**.*</AdditionalFilesToInclude>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the resources don't have an extension. If you ignore Resources*. it will only ignore resources that have a . in them. I believe I tried Resources** alone as well, I don't remember why that one didn't work.

@nulltoken
Copy link
Member

🆒 Please rebase this on vNext and I'll get it merged.

Let's get NCrunch'd 🎉

@MicahZoltu
Copy link
Contributor Author

Done.

@nulltoken nulltoken merged commit f60ef6a into libgit2:vNext Sep 17, 2014
@nulltoken nulltoken added this to the v0.20 milestone Sep 17, 2014
@nulltoken
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants